Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(rpm): added rpm versioning functionality #26340

Merged
merged 21 commits into from
Jan 5, 2024
Merged

feat(rpm): added rpm versioning functionality #26340

merged 21 commits into from
Jan 5, 2024

Conversation

laundry-96
Copy link
Contributor

Changes

Added RPM functionality with the help of @JamieMagee.
I had to reset the changes due to conflicts, but Jamie did most of the work so mentioning them for credit.

Context

Closes #20465

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@laundry-96 laundry-96 changed the title feat(rpm) Added RPM versioning functionality feat(rpm) added rpm versioning functionality Dec 17, 2023
@laundry-96 laundry-96 changed the title feat(rpm) added rpm versioning functionality feat(rpm): added rpm versioning functionality Dec 17, 2023
rarkins
rarkins previously approved these changes Dec 17, 2023
View reviewed changes
@rarkins rarkins enabled auto-merge December 17, 2023 05:48
viceice
viceice reviewed Dec 17, 2023
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing coverage

@laundry-96
Copy link
Contributor Author

@viceice How can I fix the coverage check?
I have read the development docs, but I only see that I need 100% coverage, and checking coverage/index.html that I need to cover some more branches (I'm assuming if statements).

If you have any more docs or some other code I can reference, that would be very helpful! And thank you so much for taking time out of your weekend to check :)

auto-merge was automatically disabled December 18, 2023 09:08

Head branch was pushed to by a user without write access

@CLAassistant
Copy link

CLAassistant commented Dec 18, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@laundry-96
Copy link
Contributor Author

There were some tests in index.spec.ts that I fixed by finding the correct value by using rpm.labelCompare in Python.

I also fixed the issues coverage, and added more functionality that is supported by RPM for full coverage.
I have learned more than I ever wanted about RPM versioning, and I now realize why no one has implemented this yet 🥲

rarkins
rarkins previously approved these changes Dec 18, 2023
View reviewed changes
viceice
viceice reviewed Dec 18, 2023
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise LGTM

@laundry-96
Copy link
Contributor Author

@viceice Accepted! Thank you :)

viceice
viceice reviewed Dec 18, 2023
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, found some more.

@laundry-96
Copy link
Contributor Author

@viceice All good? and no problems! Glad someone can check :)

@laundry-96
Copy link
Contributor Author

@viceice The Number.parseInt doesn't work because the lines above remove leading zeros. If a string is '0' it turns into '', which that function returns NaN.

viceice
viceice reviewed Dec 26, 2023
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment

@rarkins rarkins enabled auto-merge January 4, 2024 04:04
@laundry-96 laundry-96 requested a review from viceice January 4, 2024 06:23
@viceice viceice requested a review from rarkins January 5, 2024 10:38
@rarkins rarkins added this pull request to the merge queue Jan 5, 2024
Merged via the queue into renovatebot:main with commit 0c92669 Jan 5, 2024
34 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.124.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

zT-1337 pushed a commit to scm-manager/renovate that referenced this pull request Jan 22, 2024
@laundry-96 @viceice @zT-1337
feat(rpm): added rpm versioning functionality (renovatebot#26340)
c232b04 
Co-authored-by: Michael Kriese <[email protected]>
zT-1337 pushed a commit to scm-manager/renovate that referenced this pull request Jan 24, 2024
@laundry-96 @viceice @zT-1337
feat(rpm): added rpm versioning functionality (renovatebot#26340)
1b31c20 
Co-authored-by: Michael Kriese <[email protected]>
zT-1337 pushed a commit to scm-manager/renovate that referenced this pull request Jan 24, 2024
@laundry-96 @viceice @zT-1337
feat(rpm): added rpm versioning functionality (renovatebot#26340)
65d49a3 
Co-authored-by: Michael Kriese <[email protected]>
@lmilbaum
Copy link
Contributor

How can this manager be used? Didn't find documentation for it. Thanks.

@viceice
Copy link
Member

viceice commented Jan 29, 2024

How can this manager be used? Didn't find documentation for it. Thanks.

please open a new discussion.

this is a versioning, not a manager 😉

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@viceice viceice viceice approved these changes

@rarkins rarkins Awaiting requested review from rarkins

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add RPM versioning
6 participants
@laundry-96 @CLAassistant @renovate-release @lmilbaum @viceice @rarkins